Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit number of shadow casting lights in ogre2 #155

Merged
merged 2 commits into from
Oct 5, 2020
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Oct 1, 2020

It was found that when the number of shadow casting lights exceed a certain number, the dynamically generated shaders produce a compile error that suggests it's reached the max number of uniform variables allowed. This causes an rendering issue where parts of the scene becomes dark. So the workaround here is to put a hard limit on the max number of lights allowed in ogre2.

Here's a max_shadows.sdf world file with lights for testing. Before the fix, the model in the world will be dark and you'll see the compiler error in ~/.ignition/rendering/ogre2.log. After the fix, the model should render fine.

Signed-off-by: Ian Chen [email protected]

@iche033 iche033 requested a review from nkoenig October 1, 2020 03:23
@github-actions github-actions bot added the 📜 blueprint Ignition Blueprint label Oct 1, 2020
@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #155 into ign-rendering2 will decrease coverage by 0.03%.
The diff coverage is 20.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering2     #155      +/-   ##
==================================================
- Coverage           51.42%   51.39%   -0.04%     
==================================================
  Files                 117      117              
  Lines               10078    10088      +10     
==================================================
+ Hits                 5183     5185       +2     
- Misses               4895     4903       +8     
Impacted Files Coverage Δ
ogre2/src/Ogre2RenderTarget.cc 72.46% <20.00%> (-1.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f049ce...7b84199. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I just think we should warn the user about what's happening.

ogre2/src/Ogre2RenderTarget.cc Outdated Show resolved Hide resolved
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 merged commit 7253594 into ign-rendering2 Oct 5, 2020
@iche033 iche033 deleted the num_shadows branch October 5, 2020 22:20
@peci1
Copy link
Contributor

peci1 commented Nov 20, 2020

Is this actually somehow discussed with OGRE people? It seems that 25 light sources is pretty easy to get with the numbers of robots allowed...

@iche033
Copy link
Contributor Author

iche033 commented Nov 21, 2020

Not yet. I should check to see if it's still an issue in the latest version of OGRE 2.x. It's probably rare for games to have that many shadows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants